Skip to content

Conversation

@HeartLinked
Copy link
Contributor

  • Fix ManifestEntry::operator== logic with proper parentheses grouping
  • Add null pointer safety in DataFile::Type() for non-partitioned tables
  • Change partition field type validation from list to struct in manifest reader
  • Support empty partition structs for non-partitioned tables (n_children == 0)
  • Update unit test for manifest file: separate v1 and v2 test, and add another v2 test for non-partitioned tables

@HeartLinked HeartLinked force-pushed the manifest-reader-v2-fixes branch from 8db6621 to 76994af Compare August 14, 2025 06:06
@HeartLinked HeartLinked changed the title fix: correct partition field handling for non-partitioned tables and add test fix: correct partition field handling for non-partitioned tables and add test for manifest file reader Aug 14, 2025
Copy link
Member

@wgtmac wgtmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- Add V2NonPartitionedBasicTest to test manifest reader v2 functionality
- Fix bounds values to match actual manifest file data
- Use proper C++23 designated initializers for test data setup
- Fix ManifestEntry::operator== logic with proper parentheses grouping
- Add null pointer safety in DataFile::Type() for non-partitioned tables
- Change partition field type validation from list to struct in manifest reader
- Support empty partition structs for non-partitioned tables (n_children == 0)
- Update test bounds values to match actual manifest file data

This resolves issues with non-partitioned table support and fixes
critical bugs in object comparison and type validation.
@HeartLinked HeartLinked force-pushed the manifest-reader-v2-fixes branch from a2bb3cb to 4be162f Compare August 15, 2025 09:57
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this!

@Xuanwo Xuanwo merged commit f2d0abd into apache:main Aug 19, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants